src/components/Advertisements/core/AdvertisementEntry/AdvertisementEntry.tsx (3)

Line range hint 77-81: Specify the type attribute for the button to ensure correct behavior in forms.

+ type="button"

---

Line range hint 99-99: Add keyboard accessibility to the delete button by handling key events.

+ onKeyUp={(event) => { if (event.key === 'Enter') toggleShowDeleteModal(); }}

---

Line range hint 147-147: Self-close JSX elements that don't have children.

- <Card.Img className={styles.admedia} variant="top" src={mediaUrl} data-testid="media"></Card.Img>
+ <Card.Img className={styles.admedia} variant="top" src={mediaUrl} data-testid="media" />

Also applies to: 158-158

src/components/AddOn/core/AddOnStore/AddOnStore.tsx (1)

Line range hint 75-75: Self-close JSX elements that don't have children.

- <div className={styles.loader}></div>
+ <div className={styles.loader} />

src/components/Advertisements/Advertisements.tsx (3)

Line range hint 53-53: Use optional chaining to safely access nested properties.

- orgAdvertisementListData && orgAdvertisementListData.organizations
+ orgAdvertisementListData?.organizations

Also applies to: 67-67

---

Line range hint 126-128: Self-close JSX elements that don't have children.

- <div className={styles.itemCard}></div>
+ <div className={styles.itemCard} />

Also applies to: 130-130, 205-207, 209-209

---

Line range hint 123-123: Avoid using the index of an array as a key in React lists.

- key={index}
+ key={`ad-${index}`}

Also applies to: 176-176, 202-202, 255-255

src/components/Advertisements/core/AdvertisementRegister/AdvertisementRegister.tsx (5)

Line range hint 217-217: Self-close JSX elements that don't have children.

- <video></video>
+ <video />

Also applies to: 312-312

---

Line range hint 221-221: Add keyboard accessibility to interactive elements by handling key events.

+ onKeyUp={(event) => { if (event.key === 'Enter') handleShow(); }}

---

Line range hint 267-267: Use optional chaining to safely access nested properties.

- if (error instanceof Error) {
+ if (error?.message) {

---

Line range hint 292-293: Provide alt text for images to improve accessibility.

- <img src={formState.advertisementMedia} />
+ <img src={formState.advertisementMedia} alt="Advertisement Media" />

---

Line range hint 295-311: Specify the type attribute for buttons to ensure correct behavior in forms.

+ type="button"

Configuration used: .coderabbit.yaml
Review profile: CHILL

src/components/AddOn/core/AddOnEntry/AddOnEntry.tsx (3)

62-62: Remove commented console.log statement

Debug statements should be removed before committing to production code.

- // console.log(currentOrg);

---

105-108: Move inline styles to CSS module

Consider moving the Card's inline styles to the existing AddOnEntry.module.css file for better maintainability and consistency.

- <Card
-   data-testid="AddOnEntry"
-   style={{ border: '1px solid #31BB6B', borderRadius: '10px' }}
- >
+ <Card
+   data-testid="AddOnEntry"
+   className={styles.addOnCard}
+ >

Add to AddOnEntry.module.css:

.addOnCard {
  border: 1px solid #31BB6B;
  border-radius: 10px;
}

---

121-121: Move title font weight to CSS module

Consider moving the Card.Title's inline style to the CSS module for consistency.

- <Card.Title style={{ fontWeight: '800' }}>{title}</Card.Title>
+ <Card.Title className={styles.cardTitle}>{title}</Card.Title>

Add to AddOnEntry.module.css:

.cardTitle {
  font-weight: 800;
}

src/components/Advertisements/Advertisements.tsx (2)

103-103: Use translation for placeholder text

The placeholder text should use the translation system for consistency with the rest of the application.

-placeholder={'Search..'}
+placeholder={t('searchPlaceholder')}

---

Line range hint 126-275: Reduce code duplication

The InfiniteScroll implementation and date filtering logic is duplicated between the active and archived tabs. Consider:

1. Creating a reusable component for the InfiniteScroll implementation
2. Extracting the date comparison logic into a utility function

Example refactor for the date logic:

const isAdvertisementActive = (ad: Ad): boolean => new Date(ad.endDate) > new Date();
const isAdvertisementArchived = (ad: Ad): boolean => !isAdvertisementActive(ad);

// Usage in JSX:
advertisements.filter(isAdvertisementActive).map(...)
advertisements.filter(isAdvertisementArchived).map(...)

src/components/AddOn/core/AddOnStore/AddOnStore.tsx (2)

45-45: Remove unused state variable for cleaner code.

The state variable dataList is not used anywhere in the code. Removing it can simplify the component and reduce unnecessary state management.

Apply this diff to remove the unused state variable:

- const [, setDataList] = useState<InterfacePluginHelper[]>([]);
+ // Removed unused state variable

---

232-232: Use strict equality checks (===) for accurate comparisons.

Replace == with === when comparing searchText to ensure strict type comparison and avoid unexpected behavior.

Apply this diff:

- if (searchText == '') {
+ if (searchText === '') {

Also applies to lines: 176, 233

Configuration used: .coderabbit.yaml
Review profile: CHILL

#!/bin/bash
# Check if the translation key exists in the advertisement namespace
fd -e json . public/locales | xargs rg '"endOfResults"'

-eventKey="archievedAds"
+eventKey="archivedAds"

src/components/AddOn/core/AddOnStore/AddOnStore.test.tsx (2)

323-334: LGTM! Consider enhancing test coverage.

The updated test assertions correctly reflect the new dropdown-based filter UI. However, consider adding these test scenarios for more comprehensive coverage:

• Verify that selecting different filter options updates the displayed plugins
• Test edge cases like rapid toggling of the dropdown
• Ensure proper keyboard navigation support

Add these test cases:

// Test filter functionality
test('filters plugins correctly when dropdown option is selected', async () => {
  // ... setup code ...
  
  const dropdownButton = screen.getByRole('button', { name: /enabled/i });
  userEvent.click(dropdownButton);
  
  // Select "Disabled" option
  userEvent.click(screen.getByText(/disabled/i));
  
  // Verify only disabled plugins are shown
  expect(screen.queryByText('EnabledPlugin')).not.toBeInTheDocument();
  expect(screen.getByText('DisabledPlugin')).toBeInTheDocument();
});

// Test keyboard navigation
test('supports keyboard navigation in dropdown', async () => {
  // ... setup code ...
  
  const dropdownButton = screen.getByRole('button', { name: /enabled/i });
  
  // Open dropdown with Enter key
  dropdownButton.focus();
  fireEvent.keyDown(dropdownButton, { key: 'Enter' });
  
  // Verify dropdown options are focusable
  expect(screen.getByText(/disabled/i)).toHaveFocus();
});

---

Line range hint 1-424: Consider enhancing overall test structure and coverage.

While the test file is well-organized, consider these improvements for better maintainability and coverage:

1. Extract mock data to a separate file to improve readability
2. Add more varied test cases for error scenarios
3. Enhance the loading test to verify loading states for both tabs

Here's a suggested approach for the loading test:

test('displays loading states correctly', async () => {
  const mocks = [ORGANIZATIONS_LIST_MOCK, PLUGIN_LOADING_MOCK];
  render(
    // ... setup code ...
  );

  // Verify initial loading state
  expect(screen.getByTestId('AddOnEntryStore')).toBeInTheDocument();
  expect(screen.getByRole('progressbar')).toBeInTheDocument();

  // Switch to Installed tab
  userEvent.click(screen.getByText('Installed'));
  expect(screen.getByRole('progressbar')).toBeInTheDocument();

  // Wait for loading to complete
  await wait();
  expect(screen.queryByRole('progressbar')).not.toBeInTheDocument();
});

Consider moving mock data to a separate file:

// __mocks__/addOnStore.mocks.ts
export const PLUGIN_GET_MOCK = {
  // ... existing mock data ...
};

export const ORGANIZATIONS_LIST_MOCK = {
  // ... existing mock data ...
};

src/components/Advertisements/Advertisements.test.tsx (4)

464-464: Consider making date validation tests more robust.

The change from date[1] to date[0] suggests a dependency on array order. Consider making the test more resilient by:

1. Adding explicit assertions about the expected number of date elements
2. Using a more specific test ID or data attribute to target the exact date element

-    const dateString = date[0].innerHTML;
+    expect(date).toHaveLength(2); // Verify expected number of dates
+    const dateString = date.find(el => el.getAttribute('data-type') === 'end-date')?.innerHTML;
+    expect(dateString).toBeDefined(); // Ensure we found the correct date element

---

Line range hint 676-677: Remove console.log statements from test code.

Debug logging statements should not be committed to test files as they add noise to test output.

-    console.log('before scroll', moreiconbtn);
-    fireEvent.scroll(window, { target: { scrollY: 500 } });
+    fireEvent.scroll(window, { target: { scrollY: 500 } });

---

Line range hint 679-679: Remove console.log statement.

Debug logging statement should be removed.

-    console.log('after scroll', moreiconbtn);

---

Line range hint 462-482: Consider using a mock date to make tests deterministic.

The test relies on comparing dates with the current time, which can make tests non-deterministic. Consider using a mocked date for consistent test results.

+    // Mock current date to ensure consistent test results
+    const mockCurrentDate = new Date('2024-01-01');
+    jest.spyOn(global, 'Date').mockImplementation(() => mockCurrentDate);

     const date = await screen.findAllByTestId('Ad_end_date');
     const dateString = date[0].innerHTML;
     const dateMatch = dateString.match(
       /\b(?:Sun|Mon|Tue|Wed|Thu|Fri|Sat)\s+(Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec)\s+(\d{1,2})\s+(\d{4})\b/,
     );
     let dateObject = new Date();

     if (dateMatch) {
       const monthName = dateMatch[1];
       const day = parseInt(dateMatch[2], 10);
       const year = parseInt(dateMatch[3], 10);

       const monthIndex =
         'JanFebMarAprMayJunJulAugSepOctNovDec'.indexOf(monthName) / 3;

       dateObject = new Date(year, monthIndex, day);
     }

-    expect(dateObject.getTime()).toBeLessThan(new Date().getTime());
+    expect(dateObject.getTime()).toBeLessThan(mockCurrentDate.getTime());
+
+    // Restore Date after test
+    jest.restoreAllMocks();

Configuration used: .coderabbit.yaml
Review profile: CHILL

src/components/AddOn/core/AddOnStore/AddOnStore.tsx (2)

102-113: Enhance robustness of plugin filtering.

The current implementation has potential issues with null safety and case handling.

Consider this improved implementation:

 const filterPlugins = (
   plugins: InterfacePluginHelper[],
   searchTerm: string,
 ): InterfacePluginHelper[] => {
-  if (!searchTerm) {
+  if (!searchTerm.trim()) {
     return plugins;
   }
 
+  const normalizedSearch = searchTerm.toLowerCase().trim();
   return plugins.filter((plugin) =>
-    plugin.pluginName?.toLowerCase().includes(searchTerm.toLowerCase()),
+    plugin.pluginName.toLowerCase().includes(normalizedSearch),
   );
 };

---

126-141: Move inline styles to CSS modules.

Inline styles make the code harder to maintain and don't support media queries or pseudo-classes. Consider moving these styles to your CSS module.

Move the styles to AddOnStore.module.css:

.container {
  background-color: white;
  margin: 2px;
  padding: 10px;
  border-radius: 20px;
}

.headerCol {
  display: flex;
  align-items: center;
  justify-content: space-between;
}

Then update the JSX:

- <Row
-   className=""
-   style={{
-     backgroundColor: 'white',
-     margin: '2px',
-     padding: '10px',
-     borderRadius: '20px',
-   }}
- >
+ <Row className={styles.container}>
-   <Col
-     style={{
-       display: 'flex',
-       alignItems: 'center',
-       justifyContent: 'space-between',
-     }}
-   >
+   <Col className={styles.headerCol}>

Configuration used: .coderabbit.yaml
Review profile: CHILL

.github/workflows/codeql-codescan.yml (1)

37-37: Fix YAML indentation for consistency.

The with block should be indented with 11 spaces to maintain consistent alignment with other blocks in the workflow.

Apply this fix:

         uses: github/codeql-action/init@v3
-         with:
+          with:
           languages: ${{ matrix.language }}

🧰 Tools

🪛 yamllint

[warning] 37-37: wrong indentation: expected 11 but found 10

(indentation)

.github/workflows/push.yml (1)

56-56: Consider separating CI/CD changes into a dedicated PR.

This workflow change appears unrelated to the PR's stated objective of redesigning the advertisement and plugin screens. To maintain clear change boundaries and simplify review/testing, consider moving this CI/CD update to a separate PR focused on workflow improvements.

.github/workflows/pull-request.yml (1)

Line range hint 209-213: Consider type checking dependencies of changed files

While compiling only changed TypeScript files is more efficient, it might miss type errors in dependent files. Consider:

1. Using TypeScript's --build mode to include dependent files
2. Running full type checks periodically

-for file in ${{ steps.changed-files.outputs.all_changed_files }}; do
-  if [[ "$file" == *.ts || "$file" == *.tsx ]]; then
-    npx tsc --noEmit "$file"
-  fi
-done
+# Create a temporary tsconfig for changed files
+echo '{
+  "extends": "./tsconfig.json",
+  "files": []
+}' > tsconfig.changed.json
+
+# Add changed files to the config
+for file in ${{ steps.changed-files.outputs.all_changed_files }}; do
+  if [[ "$file" == *.ts || "$file" == *.tsx ]]; then
+    echo $(jq --arg file "$file" '.files += [$file]' tsconfig.changed.json) > tsconfig.changed.json
+  fi
+done
+
+# Build with dependencies
+npx tsc --build tsconfig.changed.json

🧰 Tools

🪛 actionlint

208-208: shellcheck reported issue in this script: SC2043:warning:1:13: This loop will only ever run once. Bad quoting or missing glob/expansion?

(shellcheck)

.github/workflows/push.yml (1)

56-56: 💡 Codebase verification

Update all Codecov action references to v5 for consistency

Found inconsistent versions of Codecov action across workflow files:

• .github/workflows/push.yml: Using v5
• .github/workflows/pull-request.yml: Using v4

The v5 action is compatible with the current setup as it doesn't introduce breaking changes in the required inputs. However, using different versions across workflows may lead to inconsistent coverage reporting behavior.

🔗 Analysis chain

Verify compatibility with Codecov action v5.

The upgrade from codecov/codecov-action@v4 to v5 is a major version change that could introduce breaking changes.

Let's verify the requirements and changes:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check Codecov action version compatibility and requirements

# Get the current Node.js version from the workflow
NODE_VERSION=$(grep 'node-version:' .github/workflows/push.yml | head -n 1 | awk -F'[' '{print $2}' | tr -d '[]')

# Fetch Codecov action v5 requirements
echo "Checking Codecov action v5 requirements..."
curl -s https://raw.githubusercontent.com/codecov/codecov-action/v5/action.yml

# Check if there are any other Codecov action usages that need to be updated
echo "Checking for other Codecov action usages..."
rg "codecov/codecov-action@v" .github/workflows/

Length of output: 11452

src/components/AddOn/core/AddOnStore/AddOnStore.tsx (2)

100-111: Improve null safety in filterPlugins function.

The function should handle cases where pluginName is undefined more gracefully.

const filterPlugins = (
  plugins: InterfacePluginHelper[],
  searchTerm: string,
): InterfacePluginHelper[] => {
  if (!searchTerm) {
    return plugins;
  }

  return plugins.filter((plugin) =>
-   plugin.pluginName?.toLowerCase().includes(searchTerm.toLowerCase()),
+   plugin.pluginName?.toLowerCase().includes(searchTerm.toLowerCase()) ?? false,
  );
};

---

125-139: Move inline styles to CSS module.

Consider moving the inline styles to the CSS module for better maintainability and consistency with the existing styling approach.

- <Row
-   className=""
-   style={{
-     backgroundColor: 'white',
-     margin: '2px',
-     padding: '10px',
-     borderRadius: '20px',
-   }}
- >
+ <Row className={styles.storeContainer}>

- <Col
-   style={{
-     display: 'flex',
-     alignItems: 'center',
-     justifyContent: 'space-between',
-   }}
- >
+ <Col className={styles.storeHeader}>

Add to AddOnStore.module.css:

.storeContainer {
  background-color: white;
  margin: 2px;
  padding: 10px;
  border-radius: 20px;
}

.storeHeader {
  display: flex;
  align-items: center;
  justify-content: space-between;
}

Configuration used: .coderabbit.yaml
Review profile: CHILL

interface InterfacePluginHelper {
  _id: string;
- pluginName?: string;
- pluginDesc?: string;
+ pluginName: string;  // Required as it's used in rendering
+ pluginDesc: string;  // Required as it's used in rendering
  pluginCreatedBy: string;
- pluginInstallStatus?: boolean;
+ pluginInstallStatus: boolean;
  uninstalledOrgs: string[];
  installed: boolean;
  enabled: boolean;
- name: string;      // Remove redundant field
- component: string;
+ component: 'Special Component';  // Use literal type
}

interface PluginListProps {
  plugins: InterfacePluginHelper[];
  getInstalledPlugins: () => void;
}

const PluginList: React.FC<PluginListProps> = ({ plugins, getInstalledPlugins }) => (
  <div className={styles.justifysp}>
    {plugins.map((plug) => (
      <div className={styles.cardGridItem} key={plug._id}>
        <AddOnEntry
          id={plug._id}
          title={plug.pluginName}
          description={plug.pluginDesc}
          createdBy={plug.pluginCreatedBy}
          component={'Special Component'}
          modified={true}
          getInstalledPlugins={getInstalledPlugins}
          uninstalledOrgs={plug.uninstalledOrgs}
        />
      </div>
    ))}
  </div>
);